[PATCH 0/7] imx-drm: additional alpha transparency formats

[PATCH 0/7] imx-drm: additional alpha transparency formats

[Philipp Zabel]

Hi,

this series adds support for more alpha transpancy formats such as
RGBA 8:8:8:8, ARGB 1:5:5:5, and the more esoteric formats with 8-bit
alpha on a separate plane.

For the latter, new 2-plane RGB formats are added:
    DRM_FORMAT_XRGB8888_A8
    DRM_FORMAT_XBGR8888_A8
    DRM_FORMAT_RGBX8888_A8
    DRM_FORMAT_BGRX8888_A8
    DRM_FORMAT_RGB888_A8
    DRM_FORMAT_BGR888_A8
    DRM_FORMAT_RGB565_A8
    DRM_FORMAT_BGR565_A8

They contain RGB data in the first plane and 8-bit alpha values in the
second plane. If supported by the hardware, memory bandwidth usage can
be reduced in case of frame buffers with large, completely transparent
regions by selectively skipping read accesses in the color channel.

regards
Philipp

Philipp Zabel (7):
  gpu: ipu-v3: Add support for 15-bit RGB with 1-bit alpha formats
  drm/imx: Add support for 15-bit RGB with 1-bit alpha formats
  gpu: ipu-v3: add support for RGBX8888 and RGBA8888 pixel formats
  gpu: ipu-v3: add support for separate alpha channels
  drm/imx: ipuv3-plane: enable support for RGBX8888 and RGBA8888 pixel
    formats
  drm: add RGB formats with separate alpha plane
  drm/imx: ipuv3-plane: add support for separate alpha planes

 drivers/gpu/drm/drm_crtc.c        |  35 ++++++++++
 drivers/gpu/drm/imx/ipuv3-plane.c |  86 ++++++++++++++++++++++++
 drivers/gpu/drm/imx/ipuv3-plane.h |   2 +
 drivers/gpu/ipu-v3/ipu-common.c   |  10 +++
 drivers/gpu/ipu-v3/ipu-cpmem.c    | 133 ++++++++++++++++++++++++++++++++++++--
 include/uapi/drm/drm_fourcc.h     |  14 ++++
 include/video/imx-ipu-v3.h        |  22 +++++++
 7 files changed, 298 insertions(+), 4 deletions(-)

-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/7] gpu: ipu-v3: Add support for 15-bit RGB with 1-bit alpha formats

[Philipp Zabel]

This patch adds support for ARGB1555, ABGR1555, RGBA5551, and BGRA5551
in-memory formats.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/ipu-v3/ipu-common.c |  4 ++++
 drivers/gpu/ipu-v3/ipu-cpmem.c  | 44 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
index 67bab5c..e21a358 100644
--- a/drivers/gpu/ipu-v3/ipu-common.c
+++ b/drivers/gpu/ipu-v3/ipu-common.c
@@ -57,6 +57,10 @@ EXPORT_SYMBOL_GPL(ipu_srm_dp_sync_update);
 enum ipu_color_space ipu_drm_fourcc_to_colorspace(u32 drm_fourcc)
 {
 	switch (drm_fourcc) {
+	case DRM_FORMAT_ARGB1555:
+	case DRM_FORMAT_ABGR1555:
+	case DRM_FORMAT_RGBA5551:
+	case DRM_FORMAT_BGRA5551:
 	case DRM_FORMAT_RGB565:
 	case DRM_FORMAT_BGR565:
 	case DRM_FORMAT_RGB888:
diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
index 3bf05bc..d26b8be 100644
--- a/drivers/gpu/ipu-v3/ipu-cpmem.c
+++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
@@ -500,6 +500,38 @@ static const struct ipu_rgb def_bgr_16 = {
 	.bits_per_pixel = 16,
 };
 
+static const struct ipu_rgb def_argb_16 = {
+	.red	= { .offset = 10, .length = 5, },
+	.green	= { .offset =  5, .length = 5, },
+	.blue	= { .offset =  0, .length = 5, },
+	.transp = { .offset = 15, .length = 1, },
+	.bits_per_pixel = 16,
+};
+
+static const struct ipu_rgb def_abgr_16 = {
+	.red	= { .offset =  0, .length = 5, },
+	.green	= { .offset =  5, .length = 5, },
+	.blue	= { .offset = 10, .length = 5, },
+	.transp = { .offset = 15, .length = 1, },
+	.bits_per_pixel = 16,
+};
+
+static const struct ipu_rgb def_rgba_16 = {
+	.red	= { .offset = 11, .length = 5, },
+	.green	= { .offset =  6, .length = 5, },
+	.blue	= { .offset =  1, .length = 5, },
+	.transp = { .offset =  0, .length = 1, },
+	.bits_per_pixel = 16,
+};
+
+static const struct ipu_rgb def_bgra_16 = {
+	.red	= { .offset =  1, .length = 5, },
+	.green	= { .offset =  6, .length = 5, },
+	.blue	= { .offset = 11, .length = 5, },
+	.transp = { .offset =  0, .length = 1, },
+	.bits_per_pixel = 16,
+};
+
 #define Y_OFFSET(pix, x, y)	((x) + pix->width * (y))
 #define U_OFFSET(pix, x, y)	((pix->width * pix->height) +		\
 				 (pix->width * (y) / 4) + (x) / 2)
@@ -581,6 +613,18 @@ int ipu_cpmem_set_fmt(struct ipuv3_channel *ch, u32 drm_fourcc)
 	case DRM_FORMAT_BGR565:
 		ipu_cpmem_set_format_rgb(ch, &def_bgr_16);
 		break;
+	case DRM_FORMAT_ARGB1555:
+		ipu_cpmem_set_format_rgb(ch, &def_argb_16);
+		break;
+	case DRM_FORMAT_ABGR1555:
+		ipu_cpmem_set_format_rgb(ch, &def_abgr_16);
+		break;
+	case DRM_FORMAT_RGBA5551:
+		ipu_cpmem_set_format_rgb(ch, &def_rgba_16);
+		break;
+	case DRM_FORMAT_BGRA5551:
+		ipu_cpmem_set_format_rgb(ch, &def_bgra_16);
+		break;
 	default:
 		return -EINVAL;
 	}
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/7] drm/imx: Add support for 15-bit RGB with 1-bit alpha formats

[Philipp Zabel]

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/drm/imx/ipuv3-plane.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 878a643..d13dbb6 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -23,8 +23,12 @@
 #define to_ipu_plane(x)	container_of(x, struct ipu_plane, base)
 
 static const uint32_t ipu_plane_formats[] = {
+	DRM_FORMAT_ARGB1555,
 	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_ABGR1555,
 	DRM_FORMAT_XBGR1555,
+	DRM_FORMAT_RGBA5551,
+	DRM_FORMAT_BGRA5551,
 	DRM_FORMAT_ARGB8888,
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_ABGR8888,
@@ -175,6 +179,10 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc,
 		ipu_dp_set_window_pos(ipu_plane->dp, crtc_x, crtc_y);
 		/* Enable local alpha on partial plane */
 		switch (fb->pixel_format) {
+		case DRM_FORMAT_ARGB1555:
+		case DRM_FORMAT_ABGR1555:
+		case DRM_FORMAT_RGBA5551:
+		case DRM_FORMAT_BGRA5551:
 		case DRM_FORMAT_ARGB8888:
 		case DRM_FORMAT_ABGR8888:
 			ipu_dp_set_global_alpha(ipu_plane->dp, false, 0, false);
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/7] gpu: ipu-v3: add support for RGBX8888 and RGBA8888 pixel formats

[Philipp Zabel]

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/ipu-v3/ipu-cpmem.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
index d26b8be..0e6b868 100644
--- a/drivers/gpu/ipu-v3/ipu-cpmem.c
+++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
@@ -452,7 +452,7 @@ void ipu_cpmem_set_yuv_planar(struct ipuv3_channel *ch,
 }
 EXPORT_SYMBOL_GPL(ipu_cpmem_set_yuv_planar);
 
-static const struct ipu_rgb def_rgb_32 = {
+static const struct ipu_rgb def_xrgb_32 = {
 	.red	= { .offset = 16, .length = 8, },
 	.green	= { .offset =  8, .length = 8, },
 	.blue	= { .offset =  0, .length = 8, },
@@ -460,7 +460,7 @@ static const struct ipu_rgb def_rgb_32 = {
 	.bits_per_pixel = 32,
 };
 
-static const struct ipu_rgb def_bgr_32 = {
+static const struct ipu_rgb def_xbgr_32 = {
 	.red	= { .offset =  0, .length = 8, },
 	.green	= { .offset =  8, .length = 8, },
 	.blue	= { .offset = 16, .length = 8, },
@@ -468,6 +468,22 @@ static const struct ipu_rgb def_bgr_32 = {
 	.bits_per_pixel = 32,
 };
 
+static const struct ipu_rgb def_rgbx_32 = {
+	.red	= { .offset = 24, .length = 8, },
+	.green	= { .offset = 16, .length = 8, },
+	.blue	= { .offset =  8, .length = 8, },
+	.transp = { .offset =  0, .length = 8, },
+	.bits_per_pixel = 32,
+};
+
+static const struct ipu_rgb def_bgrx_32 = {
+	.red	= { .offset =  8, .length = 8, },
+	.green	= { .offset = 16, .length = 8, },
+	.blue	= { .offset = 24, .length = 8, },
+	.transp = { .offset =  0, .length = 8, },
+	.bits_per_pixel = 32,
+};
+
 static const struct ipu_rgb def_rgb_24 = {
 	.red	= { .offset = 16, .length = 8, },
 	.green	= { .offset =  8, .length = 8, },
@@ -595,11 +611,19 @@ int ipu_cpmem_set_fmt(struct ipuv3_channel *ch, u32 drm_fourcc)
 		break;
 	case DRM_FORMAT_ABGR8888:
 	case DRM_FORMAT_XBGR8888:
-		ipu_cpmem_set_format_rgb(ch, &def_bgr_32);
+		ipu_cpmem_set_format_rgb(ch, &def_xbgr_32);
 		break;
 	case DRM_FORMAT_ARGB8888:
 	case DRM_FORMAT_XRGB8888:
-		ipu_cpmem_set_format_rgb(ch, &def_rgb_32);
+		ipu_cpmem_set_format_rgb(ch, &def_xrgb_32);
+		break;
+	case DRM_FORMAT_RGBA8888:
+	case DRM_FORMAT_RGBX8888:
+		ipu_cpmem_set_format_rgb(ch, &def_rgbx_32);
+		break;
+	case DRM_FORMAT_BGRA8888:
+	case DRM_FORMAT_BGRX8888:
+		ipu_cpmem_set_format_rgb(ch, &def_bgrx_32);
 		break;
 	case DRM_FORMAT_BGR888:
 		ipu_cpmem_set_format_rgb(ch, &def_bgr_24);
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/7] gpu: ipu-v3: add support for separate alpha channels

[Philipp Zabel]

The IPUv3 can read 8-bit alpha values from a separate IDMAC channel driven
by the Alpha Transparency Controller (ATC) for the graphics IDMAC channels.
This allows to reduce memory bandwidth via a conditional read mechanism or
to support planar YUV formats with alpha transparency.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/ipu-v3/ipu-common.c |  6 +++++
 drivers/gpu/ipu-v3/ipu-cpmem.c  | 57 +++++++++++++++++++++++++++++++++++++++++
 include/video/imx-ipu-v3.h      | 22 ++++++++++++++++
 3 files changed, 85 insertions(+)

diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
index e21a358..72994b0 100644
--- a/drivers/gpu/ipu-v3/ipu-common.c
+++ b/drivers/gpu/ipu-v3/ipu-common.c
@@ -73,6 +73,12 @@ enum ipu_color_space ipu_drm_fourcc_to_colorspace(u32 drm_fourcc)
 	case DRM_FORMAT_ABGR8888:
 	case DRM_FORMAT_RGBA8888:
 	case DRM_FORMAT_BGRA8888:
+	case DRM_FORMAT_RGB565_A8:
+	case DRM_FORMAT_BGR565_A8:
+	case DRM_FORMAT_RGB888_A8:
+	case DRM_FORMAT_BGR888_A8:
+	case DRM_FORMAT_RGBX8888_A8:
+	case DRM_FORMAT_BGRX8888_A8:
 		return IPUV3_COLORSPACE_RGB;
 	case DRM_FORMAT_YUYV:
 	case DRM_FORMAT_UYVY:
diff --git a/drivers/gpu/ipu-v3/ipu-cpmem.c b/drivers/gpu/ipu-v3/ipu-cpmem.c
index 0e6b868..75b5412 100644
--- a/drivers/gpu/ipu-v3/ipu-cpmem.c
+++ b/drivers/gpu/ipu-v3/ipu-cpmem.c
@@ -564,6 +564,43 @@ static const struct ipu_rgb def_bgra_16 = {
 #define UV2_OFFSET(pix, x, y)	((pix->width * pix->height) +	\
 				 (pix->width * y) + (x))
 
+#define NUM_ALPHA_CHANNELS	7
+
+/* See Table 37-12. Alpha channels mapping. */
+static int ipu_channel_albm(int ch_num)
+{
+	switch (ch_num) {
+	case IPUV3_CHANNEL_G_MEM_IC_PRP_VF:	return 0;
+	case IPUV3_CHANNEL_G_MEM_IC_PP:		return 1;
+	case IPUV3_CHANNEL_MEM_FG_SYNC:		return 2;
+	case IPUV3_CHANNEL_MEM_FG_ASYNC:	return 3;
+	case IPUV3_CHANNEL_MEM_BG_SYNC:		return 4;
+	case IPUV3_CHANNEL_MEM_BG_ASYNC:	return 5;
+	case IPUV3_CHANNEL_MEM_VDI_PLANE1_COMB: return 6;
+	default:
+		return -EINVAL;
+	}
+}
+
+static void ipu_cpmem_set_separate_alpha(struct ipuv3_channel *ch)
+{
+	struct ipu_soc *ipu = ch->ipu;
+	int albm;
+	u32 val;
+
+	albm = ipu_channel_albm(ch->num);
+	if (albm < 0)
+		return;
+
+	ipu_ch_param_write_field(ch, IPU_FIELD_ALU, 1);
+	ipu_ch_param_write_field(ch, IPU_FIELD_ALBM, albm);
+	ipu_ch_param_write_field(ch, IPU_FIELD_CRE, 1);
+
+	val = ipu_idmac_read(ipu, IDMAC_SEP_ALPHA);
+	val |= BIT(ch->num);
+	ipu_idmac_write(ipu, val, IDMAC_SEP_ALPHA);
+}
+
 int ipu_cpmem_set_fmt(struct ipuv3_channel *ch, u32 drm_fourcc)
 {
 	switch (drm_fourcc) {
@@ -619,22 +656,28 @@ int ipu_cpmem_set_fmt(struct ipuv3_channel *ch, u32 drm_fourcc)
 		break;
 	case DRM_FORMAT_RGBA8888:
 	case DRM_FORMAT_RGBX8888:
+	case DRM_FORMAT_RGBX8888_A8:
 		ipu_cpmem_set_format_rgb(ch, &def_rgbx_32);
 		break;
 	case DRM_FORMAT_BGRA8888:
 	case DRM_FORMAT_BGRX8888:
+	case DRM_FORMAT_BGRX8888_A8:
 		ipu_cpmem_set_format_rgb(ch, &def_bgrx_32);
 		break;
 	case DRM_FORMAT_BGR888:
+	case DRM_FORMAT_BGR888_A8:
 		ipu_cpmem_set_format_rgb(ch, &def_bgr_24);
 		break;
 	case DRM_FORMAT_RGB888:
+	case DRM_FORMAT_RGB888_A8:
 		ipu_cpmem_set_format_rgb(ch, &def_rgb_24);
 		break;
 	case DRM_FORMAT_RGB565:
+	case DRM_FORMAT_RGB565_A8:
 		ipu_cpmem_set_format_rgb(ch, &def_rgb_16);
 		break;
 	case DRM_FORMAT_BGR565:
+	case DRM_FORMAT_BGR565_A8:
 		ipu_cpmem_set_format_rgb(ch, &def_bgr_16);
 		break;
 	case DRM_FORMAT_ARGB1555:
@@ -653,6 +696,20 @@ int ipu_cpmem_set_fmt(struct ipuv3_channel *ch, u32 drm_fourcc)
 		return -EINVAL;
 	}
 
+	switch (drm_fourcc) {
+	case DRM_FORMAT_RGB565_A8:
+	case DRM_FORMAT_BGR565_A8:
+	case DRM_FORMAT_RGB888_A8:
+	case DRM_FORMAT_BGR888_A8:
+	case DRM_FORMAT_RGBX8888_A8:
+	case DRM_FORMAT_BGRX8888_A8:
+		ipu_ch_param_write_field(ch, IPU_FIELD_WID3, 7);
+		ipu_cpmem_set_separate_alpha(ch);
+		break;
+	default:
+		break;
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(ipu_cpmem_set_fmt);
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index 85dedca..9f5d220 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -118,6 +118,28 @@ enum ipu_channel_irq {
 #define IPUV3_CHANNEL_ROT_PP_MEM		50
 #define IPUV3_CHANNEL_MEM_BG_SYNC_ALPHA		51
 
+static inline int ipu_channel_alpha_channel(int ch_num)
+{
+	switch (ch_num) {
+	case IPUV3_CHANNEL_G_MEM_IC_PRP_VF:
+		return IPUV3_CHANNEL_G_MEM_IC_PRP_VF_ALPHA;
+	case IPUV3_CHANNEL_G_MEM_IC_PP:
+		return IPUV3_CHANNEL_G_MEM_IC_PP_ALPHA;
+	case IPUV3_CHANNEL_MEM_FG_SYNC:
+		return IPUV3_CHANNEL_MEM_FG_SYNC_ALPHA;
+	case IPUV3_CHANNEL_MEM_FG_ASYNC:
+		return IPUV3_CHANNEL_MEM_FG_ASYNC_ALPHA;
+	case IPUV3_CHANNEL_MEM_BG_SYNC:
+		return IPUV3_CHANNEL_MEM_BG_SYNC_ALPHA;
+	case IPUV3_CHANNEL_MEM_BG_ASYNC:
+		return IPUV3_CHANNEL_MEM_BG_ASYNC_ALPHA;
+	case IPUV3_CHANNEL_MEM_VDI_PLANE1_COMB:
+		return IPUV3_CHANNEL_MEM_VDI_PLANE1_COMB_ALPHA;
+	default:
+		return -EINVAL;
+	}
+}
+
 int ipu_map_irq(struct ipu_soc *ipu, int irq);
 int ipu_idmac_channel_irq(struct ipu_soc *ipu, struct ipuv3_channel *channel,
 		enum ipu_channel_irq irq);
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/7] drm/imx: ipuv3-plane: enable support for RGBX8888 and RGBA8888 pixel formats

[Philipp Zabel]

This patch allows to use the RGBX and RGBA 8:8:8:8 formats.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/drm/imx/ipuv3-plane.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index d13dbb6..d030990 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -33,6 +33,10 @@ static const uint32_t ipu_plane_formats[] = {
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_ABGR8888,
 	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_RGBA8888,
+	DRM_FORMAT_RGBX8888,
+	DRM_FORMAT_BGRA8888,
+	DRM_FORMAT_BGRA8888,
 	DRM_FORMAT_YUYV,
 	DRM_FORMAT_YVYU,
 	DRM_FORMAT_YUV420,
@@ -185,6 +189,8 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc,
 		case DRM_FORMAT_BGRA5551:
 		case DRM_FORMAT_ARGB8888:
 		case DRM_FORMAT_ABGR8888:
+		case DRM_FORMAT_RGBA8888:
+		case DRM_FORMAT_BGRA8888:
 			ipu_dp_set_global_alpha(ipu_plane->dp, false, 0, false);
 			break;
 		default:
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/7] drm: add RGB formats with separate alpha plane

[Philipp Zabel]

Some hardware can read the alpha components separately and then
conditionally fetch color components only for non-zero alpha values.
This patch adds fourcc definitions for two-plane RGB formats with an
8-bit alpha channel on a second plane.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/drm/drm_crtc.c    | 35 +++++++++++++++++++++++++++++++++++
 include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 3007b44..2ac0d7c 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3245,6 +3245,14 @@ static int format_check(const struct drm_mode_fb_cmd2 *r)
 	case DRM_FORMAT_YVU422:
 	case DRM_FORMAT_YUV444:
 	case DRM_FORMAT_YVU444:
+	case DRM_FORMAT_XRGB8888_A8:
+	case DRM_FORMAT_XBGR8888_A8:
+	case DRM_FORMAT_RGBX8888_A8:
+	case DRM_FORMAT_BGRX8888_A8:
+	case DRM_FORMAT_RGB888_A8:
+	case DRM_FORMAT_BGR888_A8:
+	case DRM_FORMAT_RGB565_A8:
+	case DRM_FORMAT_BGR565_A8:
 		return 0;
 	default:
 		DRM_DEBUG_KMS("invalid pixel format %s\n",
@@ -5198,11 +5206,15 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
 		break;
 	case DRM_FORMAT_RGB565:
 	case DRM_FORMAT_BGR565:
+	case DRM_FORMAT_RGB565_A8:
+	case DRM_FORMAT_BGR565_A8:
 		*depth = 16;
 		*bpp = 16;
 		break;
 	case DRM_FORMAT_RGB888:
 	case DRM_FORMAT_BGR888:
+	case DRM_FORMAT_RGB888_A8:
+	case DRM_FORMAT_BGR888_A8:
 		*depth = 24;
 		*bpp = 24;
 		break;
@@ -5210,6 +5222,10 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
 	case DRM_FORMAT_XBGR8888:
 	case DRM_FORMAT_RGBX8888:
 	case DRM_FORMAT_BGRX8888:
+	case DRM_FORMAT_XRGB8888_A8:
+	case DRM_FORMAT_XBGR8888_A8:
+	case DRM_FORMAT_RGBX8888_A8:
+	case DRM_FORMAT_BGRX8888_A8:
 		*depth = 24;
 		*bpp = 32;
 		break;
@@ -5268,6 +5284,14 @@ int drm_format_num_planes(uint32_t format)
 	case DRM_FORMAT_NV61:
 	case DRM_FORMAT_NV24:
 	case DRM_FORMAT_NV42:
+	case DRM_FORMAT_RGB565_A8:
+	case DRM_FORMAT_BGR565_A8:
+	case DRM_FORMAT_RGB888_A8:
+	case DRM_FORMAT_BGR888_A8:
+	case DRM_FORMAT_XRGB8888_A8:
+	case DRM_FORMAT_XBGR8888_A8:
+	case DRM_FORMAT_RGBX8888_A8:
+	case DRM_FORMAT_BGRX8888_A8:
 		return 2;
 	default:
 		return 1;
@@ -5315,6 +5339,17 @@ int drm_format_plane_cpp(uint32_t format, int plane)
 	case DRM_FORMAT_YUV444:
 	case DRM_FORMAT_YVU444:
 		return 1;
+	case DRM_FORMAT_RGB565_A8:
+	case DRM_FORMAT_BGR565_A8:
+		return plane ? 1 : 2;
+	case DRM_FORMAT_RGB888_A8:
+	case DRM_FORMAT_BGR888_A8:
+		return plane ? 1 : 3;
+	case DRM_FORMAT_XRGB8888_A8:
+	case DRM_FORMAT_XBGR8888_A8:
+	case DRM_FORMAT_RGBX8888_A8:
+	case DRM_FORMAT_BGRX8888_A8:
+		return plane ? 1 : 4;
 	default:
 		drm_fb_get_bpp_depth(format, &depth, &bpp);
 		return bpp >> 3;
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 0773582..48d6ec8 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -96,6 +96,20 @@
 #define DRM_FORMAT_AYUV		fourcc_code('A', 'Y', 'U', 'V') /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
 
 /*
+ * 2 plane RGB + A
+ * index 0 = RGB plane
+ * index 1 = A plane
+ */
+#define DRM_FORMAT_XRGB8888_A8	fourcc_code('X', 'R', 'A', '8')
+#define DRM_FORMAT_XBGR8888_A8	fourcc_code('X', 'B', 'A', '8')
+#define DRM_FORMAT_RGBX8888_A8	fourcc_code('R', 'X', 'A', '8')
+#define DRM_FORMAT_BGRX8888_A8	fourcc_code('B', 'X', 'A', '8')
+#define DRM_FORMAT_RGB888_A8	fourcc_code('R', '8', 'A', '8')
+#define DRM_FORMAT_BGR888_A8	fourcc_code('B', '8', 'A', '8')
+#define DRM_FORMAT_RGB565_A8	fourcc_code('R', '5', 'A', '8')
+#define DRM_FORMAT_BGR565_A8	fourcc_code('B', '5', 'A', '8')
+
+/*
  * 2 plane YCbCr
  * index 0 = Y plane, [7:0] Y
  * index 1 = Cr:Cb plane, [15:0] Cr:Cb little endian
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 7/7] drm/imx: ipuv3-plane: add support for separate alpha planes

[Philipp Zabel]

The IPUv3 can read 8-bit alpha values from a separate plane buffer using a
companion IDMAC channel driven by the Alpha Transparency Controller (ATC)
for the graphics channels. The conditional read mechanism allows to reduce
memory bandwidth by skipping reads of color data for completely transparent
bursts.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/gpu/drm/imx/ipuv3-plane.c | 72 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/imx/ipuv3-plane.h |  2 ++
 2 files changed, 74 insertions(+)

diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index d030990..ca10d55 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -41,6 +41,12 @@ static const uint32_t ipu_plane_formats[] = {
 	DRM_FORMAT_YVYU,
 	DRM_FORMAT_YUV420,
 	DRM_FORMAT_YVU420,
+	DRM_FORMAT_RGB565_A8,
+	DRM_FORMAT_BGR565_A8,
+	DRM_FORMAT_RGB888_A8,
+	DRM_FORMAT_BGR888_A8,
+	DRM_FORMAT_RGBX8888_A8,
+	DRM_FORMAT_BGRX8888_A8,
 };
 
 int ipu_plane_irq(struct ipu_plane *ipu_plane)
@@ -71,6 +77,7 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
 		       int x, int y)
 {
 	struct drm_gem_cma_object *cma_obj;
+	unsigned long alpha_eba = 0;
 	unsigned long eba;
 	int active;
 
@@ -86,13 +93,36 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
 	eba = cma_obj->paddr + fb->offsets[0] +
 	      fb->pitches[0] * y + (fb->bits_per_pixel >> 3) * x;
 
+	switch (fb->pixel_format) {
+	case DRM_FORMAT_RGB565_A8:
+	case DRM_FORMAT_BGR565_A8:
+	case DRM_FORMAT_RGB888_A8:
+	case DRM_FORMAT_BGR888_A8:
+	case DRM_FORMAT_RGBX8888_A8:
+	case DRM_FORMAT_BGRX8888_A8:
+		alpha_eba = cma_obj->paddr + fb->offsets[1] +
+			    fb->pitches[1] * y + x;
+		break;
+	}
+
 	if (ipu_plane->enabled) {
 		active = ipu_idmac_get_current_buffer(ipu_plane->ipu_ch);
 		ipu_cpmem_set_buffer(ipu_plane->ipu_ch, !active, eba);
 		ipu_idmac_select_buffer(ipu_plane->ipu_ch, !active);
+		if (alpha_eba) {
+			active = ipu_idmac_get_current_buffer(
+						ipu_plane->alpha_ch);
+			ipu_cpmem_set_buffer(ipu_plane->alpha_ch, !active,
+					     alpha_eba);
+			ipu_idmac_select_buffer(ipu_plane->alpha_ch, !active);
+		}
 	} else {
 		ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 0, eba);
 		ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 1, eba);
+		if (alpha_eba) {
+			ipu_cpmem_set_buffer(ipu_plane->alpha_ch, 0, alpha_eba);
+			ipu_cpmem_set_buffer(ipu_plane->alpha_ch, 1, alpha_eba);
+		}
 	}
 
 	/* cache offsets for subsequent pageflips */
@@ -163,6 +193,7 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc,
 		return ipu_plane_set_base(ipu_plane, fb, src_x, src_y);
 	}
 
+	ipu_plane->separate_alpha = false;
 	switch (ipu_plane->dp_flow) {
 	case IPU_DP_FLOW_SYNC_BG:
 		ret = ipu_dp_setup_channel(ipu_plane->dp,
@@ -183,6 +214,16 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc,
 		ipu_dp_set_window_pos(ipu_plane->dp, crtc_x, crtc_y);
 		/* Enable local alpha on partial plane */
 		switch (fb->pixel_format) {
+		case DRM_FORMAT_RGB565_A8:
+		case DRM_FORMAT_BGR565_A8:
+		case DRM_FORMAT_RGB888_A8:
+		case DRM_FORMAT_BGR888_A8:
+		case DRM_FORMAT_RGBX8888_A8:
+		case DRM_FORMAT_BGRX8888_A8:
+			if (!ipu_plane->alpha_ch)
+				return -EINVAL;
+			ipu_plane->separate_alpha = true;
+			/* fallthrough */
 		case DRM_FORMAT_ARGB1555:
 		case DRM_FORMAT_ABGR1555:
 		case DRM_FORMAT_RGBA5551:
@@ -224,6 +265,18 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc,
 	ipu_idmac_set_double_buffer(ipu_plane->ipu_ch, 1);
 	ipu_cpmem_set_stride(ipu_plane->ipu_ch, fb->pitches[0]);
 
+	if (ipu_plane->separate_alpha) {
+		ipu_cpmem_set_burstsize(ipu_plane->ipu_ch, 16);
+
+		ipu_cpmem_zero(ipu_plane->alpha_ch);
+		ipu_cpmem_set_resolution(ipu_plane->alpha_ch, src_w, src_h);
+		ipu_cpmem_set_format_passthrough(ipu_plane->alpha_ch, 8);
+		ipu_cpmem_set_high_priority(ipu_plane->alpha_ch);
+		ipu_idmac_set_double_buffer(ipu_plane->alpha_ch, 1);
+		ipu_cpmem_set_stride(ipu_plane->alpha_ch, fb->pitches[1]);
+		ipu_cpmem_set_burstsize(ipu_plane->alpha_ch, 16);
+	}
+
 	ret = ipu_plane_set_base(ipu_plane, fb, src_x, src_y);
 	if (ret < 0)
 		return ret;
@@ -244,11 +297,14 @@ void ipu_plane_put_resources(struct ipu_plane *ipu_plane)
 		ipu_dmfc_put(ipu_plane->dmfc);
 	if (!IS_ERR_OR_NULL(ipu_plane->ipu_ch))
 		ipu_idmac_put(ipu_plane->ipu_ch);
+	if (!IS_ERR_OR_NULL(ipu_plane->alpha_ch))
+		ipu_idmac_put(ipu_plane->alpha_ch);
 }
 
 int ipu_plane_get_resources(struct ipu_plane *ipu_plane)
 {
 	int ret;
+	int alpha_ch;
 
 	ipu_plane->ipu_ch = ipu_idmac_get(ipu_plane->ipu, ipu_plane->dma);
 	if (IS_ERR(ipu_plane->ipu_ch)) {
@@ -257,6 +313,18 @@ int ipu_plane_get_resources(struct ipu_plane *ipu_plane)
 		return ret;
 	}
 
+	alpha_ch = ipu_channel_alpha_channel(ipu_plane->dma);
+	if (alpha_ch >= 0) {
+		ipu_plane->alpha_ch = ipu_idmac_get(ipu_plane->ipu, alpha_ch);
+		if (IS_ERR(ipu_plane->alpha_ch)) {
+			ipu_idmac_put(ipu_plane->ipu_ch);
+			ret = PTR_ERR(ipu_plane->alpha_ch);
+			DRM_ERROR("failed to get alpha idmac channel %d: %d\n",
+				  alpha_ch, ret);
+			return ret;
+		}
+	}
+
 	ipu_plane->dmfc = ipu_dmfc_get(ipu_plane->ipu, ipu_plane->dma);
 	if (IS_ERR(ipu_plane->dmfc)) {
 		ret = PTR_ERR(ipu_plane->dmfc);
@@ -286,6 +354,8 @@ void ipu_plane_enable(struct ipu_plane *ipu_plane)
 		ipu_dp_enable(ipu_plane->ipu);
 	ipu_dmfc_enable_channel(ipu_plane->dmfc);
 	ipu_idmac_enable_channel(ipu_plane->ipu_ch);
+	if (ipu_plane->separate_alpha)
+		ipu_idmac_enable_channel(ipu_plane->alpha_ch);
 	if (ipu_plane->dp)
 		ipu_dp_enable_channel(ipu_plane->dp);
 
@@ -300,6 +370,8 @@ void ipu_plane_disable(struct ipu_plane *ipu_plane)
 
 	if (ipu_plane->dp)
 		ipu_dp_disable_channel(ipu_plane->dp);
+	if (ipu_plane->separate_alpha)
+		ipu_idmac_disable_channel(ipu_plane->alpha_ch);
 	ipu_idmac_disable_channel(ipu_plane->ipu_ch);
 	ipu_dmfc_disable_channel(ipu_plane->dmfc);
 	if (ipu_plane->dp)
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.h b/drivers/gpu/drm/imx/ipuv3-plane.h
index 9b5eff1..c8913ed 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.h
+++ b/drivers/gpu/drm/imx/ipuv3-plane.h
@@ -18,6 +18,7 @@ struct ipu_plane {
 
 	struct ipu_soc		*ipu;
 	struct ipuv3_channel	*ipu_ch;
+	struct ipuv3_channel	*alpha_ch;
 	struct dmfc_channel	*dmfc;
 	struct ipu_dp		*dp;
 
@@ -30,6 +31,7 @@ struct ipu_plane {
 	int			h;
 
 	bool			enabled;
+	bool			separate_alpha;
 };
 
 struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
-- 
2.1.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/7] drm: add RGB formats with separate alpha plane

[Daniel Vetter]

On Tue, May 19, 2015 at 06:06:00PM +0200, Philipp Zabel wrote:
> Some hardware can read the alpha components separately and then
> conditionally fetch color components only for non-zero alpha values.
> This patch adds fourcc definitions for two-plane RGB formats with an
> 8-bit alpha channel on a second plane.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/gpu/drm/drm_crtc.c    | 35 +++++++++++++++++++++++++++++++++++
>  include/uapi/drm/drm_fourcc.h | 14 ++++++++++++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 3007b44..2ac0d7c 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -3245,6 +3245,14 @@ static int format_check(const struct drm_mode_fb_cmd2 *r)
>  	case DRM_FORMAT_YVU422:
>  	case DRM_FORMAT_YUV444:
>  	case DRM_FORMAT_YVU444:
> +	case DRM_FORMAT_XRGB8888_A8:
> +	case DRM_FORMAT_XBGR8888_A8:
> +	case DRM_FORMAT_RGBX8888_A8:
> +	case DRM_FORMAT_BGRX8888_A8:
> +	case DRM_FORMAT_RGB888_A8:
> +	case DRM_FORMAT_BGR888_A8:
> +	case DRM_FORMAT_RGB565_A8:
> +	case DRM_FORMAT_BGR565_A8:
>  		return 0;
>  	default:
>  		DRM_DEBUG_KMS("invalid pixel format %s\n",
> @@ -5198,11 +5206,15 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
>  		break;
>  	case DRM_FORMAT_RGB565:
>  	case DRM_FORMAT_BGR565:
> +	case DRM_FORMAT_RGB565_A8:
> +	case DRM_FORMAT_BGR565_A8:
>  		*depth = 16;
>  		*bpp = 16;
>  		break;
>  	case DRM_FORMAT_RGB888:
>  	case DRM_FORMAT_BGR888:
> +	case DRM_FORMAT_RGB888_A8:
> +	case DRM_FORMAT_BGR888_A8:
>  		*depth = 24;
>  		*bpp = 24;
>  		break;
> @@ -5210,6 +5222,10 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
>  	case DRM_FORMAT_XBGR8888:
>  	case DRM_FORMAT_RGBX8888:
>  	case DRM_FORMAT_BGRX8888:
> +	case DRM_FORMAT_XRGB8888_A8:
> +	case DRM_FORMAT_XBGR8888_A8:
> +	case DRM_FORMAT_RGBX8888_A8:
> +	case DRM_FORMAT_BGRX8888_A8:
>  		*depth = 24;
>  		*bpp = 32;
>  		break;

Please drop the above two hunks, these functions are only for backwards
compat with drivers from the addfb1 days. Modern drivers should only use
the format tags directly. Extending the plane_cpp function like you do
below is enough.

Maybe we should add a WARN_ON(num_planes(format) != 0) to the top of this
and a comment that this is for legacy stuff only.

lgtm otherwise.
-Daniel

> @@ -5268,6 +5284,14 @@ int drm_format_num_planes(uint32_t format)
>  	case DRM_FORMAT_NV61:
>  	case DRM_FORMAT_NV24:
>  	case DRM_FORMAT_NV42:
> +	case DRM_FORMAT_RGB565_A8:
> +	case DRM_FORMAT_BGR565_A8:
> +	case DRM_FORMAT_RGB888_A8:
> +	case DRM_FORMAT_BGR888_A8:
> +	case DRM_FORMAT_XRGB8888_A8:
> +	case DRM_FORMAT_XBGR8888_A8:
> +	case DRM_FORMAT_RGBX8888_A8:
> +	case DRM_FORMAT_BGRX8888_A8:
>  		return 2;
>  	default:
>  		return 1;
> @@ -5315,6 +5339,17 @@ int drm_format_plane_cpp(uint32_t format, int plane)
>  	case DRM_FORMAT_YUV444:
>  	case DRM_FORMAT_YVU444:
>  		return 1;
> +	case DRM_FORMAT_RGB565_A8:
> +	case DRM_FORMAT_BGR565_A8:
> +		return plane ? 1 : 2;
> +	case DRM_FORMAT_RGB888_A8:
> +	case DRM_FORMAT_BGR888_A8:
> +		return plane ? 1 : 3;
> +	case DRM_FORMAT_XRGB8888_A8:
> +	case DRM_FORMAT_XBGR8888_A8:
> +	case DRM_FORMAT_RGBX8888_A8:
> +	case DRM_FORMAT_BGRX8888_A8:
> +		return plane ? 1 : 4;
>  	default:
>  		drm_fb_get_bpp_depth(format, &depth, &bpp);
>  		return bpp >> 3;
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 0773582..48d6ec8 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -96,6 +96,20 @@
>  #define DRM_FORMAT_AYUV		fourcc_code('A', 'Y', 'U', 'V') /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
>  
>  /*
> + * 2 plane RGB + A
> + * index 0 = RGB plane
> + * index 1 = A plane
> + */
> +#define DRM_FORMAT_XRGB8888_A8	fourcc_code('X', 'R', 'A', '8')
> +#define DRM_FORMAT_XBGR8888_A8	fourcc_code('X', 'B', 'A', '8')
> +#define DRM_FORMAT_RGBX8888_A8	fourcc_code('R', 'X', 'A', '8')
> +#define DRM_FORMAT_BGRX8888_A8	fourcc_code('B', 'X', 'A', '8')
> +#define DRM_FORMAT_RGB888_A8	fourcc_code('R', '8', 'A', '8')
> +#define DRM_FORMAT_BGR888_A8	fourcc_code('B', '8', 'A', '8')
> +#define DRM_FORMAT_RGB565_A8	fourcc_code('R', '5', 'A', '8')
> +#define DRM_FORMAT_BGR565_A8	fourcc_code('B', '5', 'A', '8')
> +
> +/*
>   * 2 plane YCbCr
>   * index 0 = Y plane, [7:0] Y
>   * index 1 = Cr:Cb plane, [15:0] Cr:Cb little endian
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 7/7] drm/imx: ipuv3-plane: add support for separate alpha planes

[Daniel Vetter]

On Tue, May 19, 2015 at 06:06:01PM +0200, Philipp Zabel wrote:
> The IPUv3 can read 8-bit alpha values from a separate plane buffer using a
> companion IDMAC channel driven by the Alpha Transparency Controller (ATC)
> for the graphics channels. The conditional read mechanism allows to reduce
> memory bandwidth by skipping reads of color data for completely transparent
> bursts.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/gpu/drm/imx/ipuv3-plane.c | 72 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/imx/ipuv3-plane.h |  2 ++
>  2 files changed, 74 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
> index d030990..ca10d55 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.c
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c
> @@ -41,6 +41,12 @@ static const uint32_t ipu_plane_formats[] = {
>  	DRM_FORMAT_YVYU,
>  	DRM_FORMAT_YUV420,
>  	DRM_FORMAT_YVU420,
> +	DRM_FORMAT_RGB565_A8,
> +	DRM_FORMAT_BGR565_A8,
> +	DRM_FORMAT_RGB888_A8,
> +	DRM_FORMAT_BGR888_A8,
> +	DRM_FORMAT_RGBX8888_A8,
> +	DRM_FORMAT_BGRX8888_A8,
>  };
>  
>  int ipu_plane_irq(struct ipu_plane *ipu_plane)
> @@ -71,6 +77,7 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
>  		       int x, int y)
>  {
>  	struct drm_gem_cma_object *cma_obj;
> +	unsigned long alpha_eba = 0;
>  	unsigned long eba;
>  	int active;
>  
> @@ -86,13 +93,36 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
>  	eba = cma_obj->paddr + fb->offsets[0] +
>  	      fb->pitches[0] * y + (fb->bits_per_pixel >> 3) * x;
>  
> +	switch (fb->pixel_format) {
> +	case DRM_FORMAT_RGB565_A8:
> +	case DRM_FORMAT_BGR565_A8:
> +	case DRM_FORMAT_RGB888_A8:
> +	case DRM_FORMAT_BGR888_A8:
> +	case DRM_FORMAT_RGBX8888_A8:
> +	case DRM_FORMAT_BGRX8888_A8:
> +		alpha_eba = cma_obj->paddr + fb->offsets[1] +
> +			    fb->pitches[1] * y + x;

You need to look at the 2nd cma_obj here, i.e. drm_fb_cma_get_gem_obj(fb, 1);

Yes, userspace is allowed to hand in non-matching. And given that you
you just reuse the cma helpers and don't reject framebuffers with
non-matching cma objects your current planar yuv support is also already
broken. Would be good if you could also patch modetest a bit to exercise
this ...
-Daniel

> +		break;
> +	}
> +
>  	if (ipu_plane->enabled) {
>  		active = ipu_idmac_get_current_buffer(ipu_plane->ipu_ch);
>  		ipu_cpmem_set_buffer(ipu_plane->ipu_ch, !active, eba);
>  		ipu_idmac_select_buffer(ipu_plane->ipu_ch, !active);
> +		if (alpha_eba) {
> +			active = ipu_idmac_get_current_buffer(
> +						ipu_plane->alpha_ch);
> +			ipu_cpmem_set_buffer(ipu_plane->alpha_ch, !active,
> +					     alpha_eba);
> +			ipu_idmac_select_buffer(ipu_plane->alpha_ch, !active);
> +		}
>  	} else {
>  		ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 0, eba);
>  		ipu_cpmem_set_buffer(ipu_plane->ipu_ch, 1, eba);
> +		if (alpha_eba) {
> +			ipu_cpmem_set_buffer(ipu_plane->alpha_ch, 0, alpha_eba);
> +			ipu_cpmem_set_buffer(ipu_plane->alpha_ch, 1, alpha_eba);
> +		}
>  	}
>  
>  	/* cache offsets for subsequent pageflips */
> @@ -163,6 +193,7 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc,
>  		return ipu_plane_set_base(ipu_plane, fb, src_x, src_y);
>  	}
>  
> +	ipu_plane->separate_alpha = false;
>  	switch (ipu_plane->dp_flow) {
>  	case IPU_DP_FLOW_SYNC_BG:
>  		ret = ipu_dp_setup_channel(ipu_plane->dp,
> @@ -183,6 +214,16 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc,
>  		ipu_dp_set_window_pos(ipu_plane->dp, crtc_x, crtc_y);
>  		/* Enable local alpha on partial plane */
>  		switch (fb->pixel_format) {
> +		case DRM_FORMAT_RGB565_A8:
> +		case DRM_FORMAT_BGR565_A8:
> +		case DRM_FORMAT_RGB888_A8:
> +		case DRM_FORMAT_BGR888_A8:
> +		case DRM_FORMAT_RGBX8888_A8:
> +		case DRM_FORMAT_BGRX8888_A8:
> +			if (!ipu_plane->alpha_ch)
> +				return -EINVAL;
> +			ipu_plane->separate_alpha = true;
> +			/* fallthrough */
>  		case DRM_FORMAT_ARGB1555:
>  		case DRM_FORMAT_ABGR1555:
>  		case DRM_FORMAT_RGBA5551:
> @@ -224,6 +265,18 @@ int ipu_plane_mode_set(struct ipu_plane *ipu_plane, struct drm_crtc *crtc,
>  	ipu_idmac_set_double_buffer(ipu_plane->ipu_ch, 1);
>  	ipu_cpmem_set_stride(ipu_plane->ipu_ch, fb->pitches[0]);
>  
> +	if (ipu_plane->separate_alpha) {
> +		ipu_cpmem_set_burstsize(ipu_plane->ipu_ch, 16);
> +
> +		ipu_cpmem_zero(ipu_plane->alpha_ch);
> +		ipu_cpmem_set_resolution(ipu_plane->alpha_ch, src_w, src_h);
> +		ipu_cpmem_set_format_passthrough(ipu_plane->alpha_ch, 8);
> +		ipu_cpmem_set_high_priority(ipu_plane->alpha_ch);
> +		ipu_idmac_set_double_buffer(ipu_plane->alpha_ch, 1);
> +		ipu_cpmem_set_stride(ipu_plane->alpha_ch, fb->pitches[1]);
> +		ipu_cpmem_set_burstsize(ipu_plane->alpha_ch, 16);
> +	}
> +
>  	ret = ipu_plane_set_base(ipu_plane, fb, src_x, src_y);
>  	if (ret < 0)
>  		return ret;
> @@ -244,11 +297,14 @@ void ipu_plane_put_resources(struct ipu_plane *ipu_plane)
>  		ipu_dmfc_put(ipu_plane->dmfc);
>  	if (!IS_ERR_OR_NULL(ipu_plane->ipu_ch))
>  		ipu_idmac_put(ipu_plane->ipu_ch);
> +	if (!IS_ERR_OR_NULL(ipu_plane->alpha_ch))
> +		ipu_idmac_put(ipu_plane->alpha_ch);
>  }
>  
>  int ipu_plane_get_resources(struct ipu_plane *ipu_plane)
>  {
>  	int ret;
> +	int alpha_ch;
>  
>  	ipu_plane->ipu_ch = ipu_idmac_get(ipu_plane->ipu, ipu_plane->dma);
>  	if (IS_ERR(ipu_plane->ipu_ch)) {
> @@ -257,6 +313,18 @@ int ipu_plane_get_resources(struct ipu_plane *ipu_plane)
>  		return ret;
>  	}
>  
> +	alpha_ch = ipu_channel_alpha_channel(ipu_plane->dma);
> +	if (alpha_ch >= 0) {
> +		ipu_plane->alpha_ch = ipu_idmac_get(ipu_plane->ipu, alpha_ch);
> +		if (IS_ERR(ipu_plane->alpha_ch)) {
> +			ipu_idmac_put(ipu_plane->ipu_ch);
> +			ret = PTR_ERR(ipu_plane->alpha_ch);
> +			DRM_ERROR("failed to get alpha idmac channel %d: %d\n",
> +				  alpha_ch, ret);
> +			return ret;
> +		}
> +	}
> +
>  	ipu_plane->dmfc = ipu_dmfc_get(ipu_plane->ipu, ipu_plane->dma);
>  	if (IS_ERR(ipu_plane->dmfc)) {
>  		ret = PTR_ERR(ipu_plane->dmfc);
> @@ -286,6 +354,8 @@ void ipu_plane_enable(struct ipu_plane *ipu_plane)
>  		ipu_dp_enable(ipu_plane->ipu);
>  	ipu_dmfc_enable_channel(ipu_plane->dmfc);
>  	ipu_idmac_enable_channel(ipu_plane->ipu_ch);
> +	if (ipu_plane->separate_alpha)
> +		ipu_idmac_enable_channel(ipu_plane->alpha_ch);
>  	if (ipu_plane->dp)
>  		ipu_dp_enable_channel(ipu_plane->dp);
>  
> @@ -300,6 +370,8 @@ void ipu_plane_disable(struct ipu_plane *ipu_plane)
>  
>  	if (ipu_plane->dp)
>  		ipu_dp_disable_channel(ipu_plane->dp);
> +	if (ipu_plane->separate_alpha)
> +		ipu_idmac_disable_channel(ipu_plane->alpha_ch);
>  	ipu_idmac_disable_channel(ipu_plane->ipu_ch);
>  	ipu_dmfc_disable_channel(ipu_plane->dmfc);
>  	if (ipu_plane->dp)
> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.h b/drivers/gpu/drm/imx/ipuv3-plane.h
> index 9b5eff1..c8913ed 100644
> --- a/drivers/gpu/drm/imx/ipuv3-plane.h
> +++ b/drivers/gpu/drm/imx/ipuv3-plane.h
> @@ -18,6 +18,7 @@ struct ipu_plane {
>  
>  	struct ipu_soc		*ipu;
>  	struct ipuv3_channel	*ipu_ch;
> +	struct ipuv3_channel	*alpha_ch;
>  	struct dmfc_channel	*dmfc;
>  	struct ipu_dp		*dp;
>  
> @@ -30,6 +31,7 @@ struct ipu_plane {
>  	int			h;
>  
>  	bool			enabled;
> +	bool			separate_alpha;
>  };
>  
>  struct ipu_plane *ipu_plane_init(struct drm_device *dev, struct ipu_soc *ipu,
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/7] drm: add RGB formats with separate alpha plane

[Philipp Zabel]

Hi Daniel,

thank you for the comments.

Am Dienstag, den 19.05.2015, 18:52 +0200 schrieb Daniel Vetter:
[...]
> > @@ -5198,11 +5206,15 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> >  		break;
> >  	case DRM_FORMAT_RGB565:
> >  	case DRM_FORMAT_BGR565:
> > +	case DRM_FORMAT_RGB565_A8:
> > +	case DRM_FORMAT_BGR565_A8:
> >  		*depth = 16;
> >  		*bpp = 16;
> >  		break;
> >  	case DRM_FORMAT_RGB888:
> >  	case DRM_FORMAT_BGR888:
> > +	case DRM_FORMAT_RGB888_A8:
> > +	case DRM_FORMAT_BGR888_A8:
> >  		*depth = 24;
> >  		*bpp = 24;
> >  		break;
> > @@ -5210,6 +5222,10 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> >  	case DRM_FORMAT_XBGR8888:
> >  	case DRM_FORMAT_RGBX8888:
> >  	case DRM_FORMAT_BGRX8888:
> > +	case DRM_FORMAT_XRGB8888_A8:
> > +	case DRM_FORMAT_XBGR8888_A8:
> > +	case DRM_FORMAT_RGBX8888_A8:
> > +	case DRM_FORMAT_BGRX8888_A8:
> >  		*depth = 24;
> >  		*bpp = 32;
> >  		break;
> 
> Please drop the above two hunks, these functions are only for backwards
> compat with drivers from the addfb1 days. Modern drivers should only use
> the format tags directly. Extending the plane_cpp function like you do
> below is enough.

I'll leave drm_fb_get_bpp_depth untouched.

> Maybe we should add a WARN_ON(num_planes(format) != 0) to the top of this
> and a comment that this is for legacy stuff only.

Do you mean:

-----8<-----
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5186,6 +5186,12 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
 void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
 			  int *bpp)
 {
+	/*
+	 * This function is to be used for legacy drivers only, no new formats
+	 * should be added here.
+	 */
+	WARN_ON(drm_format_num_planes(format) != 1);
+
 	switch (format) {
 	case DRM_FORMAT_C8:
 	case DRM_FORMAT_RGB332:
----->8-----

> lgtm otherwise.
> -Daniel

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 7/7] drm/imx: ipuv3-plane: add support for separate alpha planes

[Philipp Zabel]

Am Dienstag, den 19.05.2015, 18:58 +0200 schrieb Daniel Vetter:
[...]
> > @@ -86,13 +93,36 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
> >  	eba = cma_obj->paddr + fb->offsets[0] +
> >  	      fb->pitches[0] * y + (fb->bits_per_pixel >> 3) * x;
> >  
> > +	switch (fb->pixel_format) {
> > +	case DRM_FORMAT_RGB565_A8:
> > +	case DRM_FORMAT_BGR565_A8:
> > +	case DRM_FORMAT_RGB888_A8:
> > +	case DRM_FORMAT_BGR888_A8:
> > +	case DRM_FORMAT_RGBX8888_A8:
> > +	case DRM_FORMAT_BGRX8888_A8:
> > +		alpha_eba = cma_obj->paddr + fb->offsets[1] +
> > +			    fb->pitches[1] * y + x;
> 
> You need to look at the 2nd cma_obj here, i.e. drm_fb_cma_get_gem_obj(fb, 1);
> 
> Yes, userspace is allowed to hand in non-matching. And given that you
> you just reuse the cma helpers and don't reject framebuffers with
> non-matching cma objects your current planar yuv support is also already
> broken. Would be good if you could also patch modetest a bit to exercise
> this ...
> -Daniel

Ow, right. I'll have a look at this.

thanks
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/7] drm: add RGB formats with separate alpha plane

[Daniel Vetter]

On Wed, May 20, 2015 at 12:50:35PM +0200, Philipp Zabel wrote:
> Hi Daniel,
> 
> thank you for the comments.
> 
> Am Dienstag, den 19.05.2015, 18:52 +0200 schrieb Daniel Vetter:
> [...]
> > > @@ -5198,11 +5206,15 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> > >  		break;
> > >  	case DRM_FORMAT_RGB565:
> > >  	case DRM_FORMAT_BGR565:
> > > +	case DRM_FORMAT_RGB565_A8:
> > > +	case DRM_FORMAT_BGR565_A8:
> > >  		*depth = 16;
> > >  		*bpp = 16;
> > >  		break;
> > >  	case DRM_FORMAT_RGB888:
> > >  	case DRM_FORMAT_BGR888:
> > > +	case DRM_FORMAT_RGB888_A8:
> > > +	case DRM_FORMAT_BGR888_A8:
> > >  		*depth = 24;
> > >  		*bpp = 24;
> > >  		break;
> > > @@ -5210,6 +5222,10 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> > >  	case DRM_FORMAT_XBGR8888:
> > >  	case DRM_FORMAT_RGBX8888:
> > >  	case DRM_FORMAT_BGRX8888:
> > > +	case DRM_FORMAT_XRGB8888_A8:
> > > +	case DRM_FORMAT_XBGR8888_A8:
> > > +	case DRM_FORMAT_RGBX8888_A8:
> > > +	case DRM_FORMAT_BGRX8888_A8:
> > >  		*depth = 24;
> > >  		*bpp = 32;
> > >  		break;
> > 
> > Please drop the above two hunks, these functions are only for backwards
> > compat with drivers from the addfb1 days. Modern drivers should only use
> > the format tags directly. Extending the plane_cpp function like you do
> > below is enough.
> 
> I'll leave drm_fb_get_bpp_depth untouched.
> 
> > Maybe we should add a WARN_ON(num_planes(format) != 0) to the top of this
> > and a comment that this is for legacy stuff only.
> 
> Do you mean:
> 
> -----8<-----
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -5186,6 +5186,12 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
>  void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
>  			  int *bpp)
>  {
> +	/*
> +	 * This function is to be used for legacy drivers only, no new formats
> +	 * should be added here.
> +	 */
> +	WARN_ON(drm_format_num_planes(format) != 1);
> +
>  	switch (format) {
>  	case DRM_FORMAT_C8:
>  	case DRM_FORMAT_RGB332:
> ----->8-----

Yeah that looks perfect, please wrap with commit message&sob and I'll
apply.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/7] drm: add RGB formats with separate alpha plane

[Ville Syrjälä]

On Wed, May 20, 2015 at 02:58:53PM +0200, Daniel Vetter wrote:
> On Wed, May 20, 2015 at 12:50:35PM +0200, Philipp Zabel wrote:
> > Hi Daniel,
> > 
> > thank you for the comments.
> > 
> > Am Dienstag, den 19.05.2015, 18:52 +0200 schrieb Daniel Vetter:
> > [...]
> > > > @@ -5198,11 +5206,15 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> > > >  		break;
> > > >  	case DRM_FORMAT_RGB565:
> > > >  	case DRM_FORMAT_BGR565:
> > > > +	case DRM_FORMAT_RGB565_A8:
> > > > +	case DRM_FORMAT_BGR565_A8:
> > > >  		*depth = 16;
> > > >  		*bpp = 16;
> > > >  		break;
> > > >  	case DRM_FORMAT_RGB888:
> > > >  	case DRM_FORMAT_BGR888:
> > > > +	case DRM_FORMAT_RGB888_A8:
> > > > +	case DRM_FORMAT_BGR888_A8:
> > > >  		*depth = 24;
> > > >  		*bpp = 24;
> > > >  		break;
> > > > @@ -5210,6 +5222,10 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> > > >  	case DRM_FORMAT_XBGR8888:
> > > >  	case DRM_FORMAT_RGBX8888:
> > > >  	case DRM_FORMAT_BGRX8888:
> > > > +	case DRM_FORMAT_XRGB8888_A8:
> > > > +	case DRM_FORMAT_XBGR8888_A8:
> > > > +	case DRM_FORMAT_RGBX8888_A8:
> > > > +	case DRM_FORMAT_BGRX8888_A8:
> > > >  		*depth = 24;
> > > >  		*bpp = 32;
> > > >  		break;
> > > 
> > > Please drop the above two hunks, these functions are only for backwards
> > > compat with drivers from the addfb1 days. Modern drivers should only use
> > > the format tags directly. Extending the plane_cpp function like you do
> > > below is enough.
> > 
> > I'll leave drm_fb_get_bpp_depth untouched.
> > 
> > > Maybe we should add a WARN_ON(num_planes(format) != 0) to the top of this
> > > and a comment that this is for legacy stuff only.
> > 
> > Do you mean:
> > 
> > -----8<-----
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -5186,6 +5186,12 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
> >  void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> >  			  int *bpp)
> >  {
> > +	/*
> > +	 * This function is to be used for legacy drivers only, no new formats
> > +	 * should be added here.
> > +	 */
> > +	WARN_ON(drm_format_num_planes(format) != 1);
> > +
> >  	switch (format) {
> >  	case DRM_FORMAT_C8:
> >  	case DRM_FORMAT_RGB332:
> > ----->8-----
> 
> Yeah that looks perfect, please wrap with commit message&sob and I'll
> apply.

I think that's going to trigger a lot. IIRC we call this thing for
any format when constructing FBs.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/7] drm: add RGB formats with separate alpha plane

[Philipp Zabel]

Am Mittwoch, den 20.05.2015, 17:07 +0300 schrieb Ville Syrjälä:
> On Wed, May 20, 2015 at 02:58:53PM +0200, Daniel Vetter wrote:
> > On Wed, May 20, 2015 at 12:50:35PM +0200, Philipp Zabel wrote:
> > > Hi Daniel,
> > > 
> > > thank you for the comments.
> > > 
> > > Am Dienstag, den 19.05.2015, 18:52 +0200 schrieb Daniel Vetter:
> > > [...]
> > > > > @@ -5198,11 +5206,15 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> > > > >  		break;
> > > > >  	case DRM_FORMAT_RGB565:
> > > > >  	case DRM_FORMAT_BGR565:
> > > > > +	case DRM_FORMAT_RGB565_A8:
> > > > > +	case DRM_FORMAT_BGR565_A8:
> > > > >  		*depth = 16;
> > > > >  		*bpp = 16;
> > > > >  		break;
> > > > >  	case DRM_FORMAT_RGB888:
> > > > >  	case DRM_FORMAT_BGR888:
> > > > > +	case DRM_FORMAT_RGB888_A8:
> > > > > +	case DRM_FORMAT_BGR888_A8:
> > > > >  		*depth = 24;
> > > > >  		*bpp = 24;
> > > > >  		break;
> > > > > @@ -5210,6 +5222,10 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> > > > >  	case DRM_FORMAT_XBGR8888:
> > > > >  	case DRM_FORMAT_RGBX8888:
> > > > >  	case DRM_FORMAT_BGRX8888:
> > > > > +	case DRM_FORMAT_XRGB8888_A8:
> > > > > +	case DRM_FORMAT_XBGR8888_A8:
> > > > > +	case DRM_FORMAT_RGBX8888_A8:
> > > > > +	case DRM_FORMAT_BGRX8888_A8:
> > > > >  		*depth = 24;
> > > > >  		*bpp = 32;
> > > > >  		break;
> > > > 
> > > > Please drop the above two hunks, these functions are only for backwards
> > > > compat with drivers from the addfb1 days. Modern drivers should only use
> > > > the format tags directly. Extending the plane_cpp function like you do
> > > > below is enough.
> > > 
> > > I'll leave drm_fb_get_bpp_depth untouched.
> > > 
> > > > Maybe we should add a WARN_ON(num_planes(format) != 0) to the top of this
> > > > and a comment that this is for legacy stuff only.
> > > 
> > > Do you mean:
> > > 
> > > -----8<-----
> > > --- a/drivers/gpu/drm/drm_crtc.c
> > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > @@ -5186,6 +5186,12 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
> > >  void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> > >  			  int *bpp)
> > >  {
> > > +	/*
> > > +	 * This function is to be used for legacy drivers only, no new formats
> > > +	 * should be added here.
> > > +	 */
> > > +	WARN_ON(drm_format_num_planes(format) != 1);
> > > +
> > >  	switch (format) {
> > >  	case DRM_FORMAT_C8:
> > >  	case DRM_FORMAT_RGB332:
> > > ----->8-----
> > 
> > Yeah that looks perfect, please wrap with commit message&sob and I'll
> > apply.
> 
> I think that's going to trigger a lot. IIRC we call this thing for
> any format when constructing FBs.

Yes I just noticed that too, for example via:

drm_fb_cma_create -> drm_fb_cma_alloc -> drm_helper_mode_fill_fb_struct
-> drm_fb_get_bpp_depth

regards
Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/7] drm: add RGB formats with separate alpha plane

[Daniel Vetter]

On Wed, May 20, 2015 at 04:54:01PM +0200, Philipp Zabel wrote:
> Am Mittwoch, den 20.05.2015, 17:07 +0300 schrieb Ville Syrjälä:
> > On Wed, May 20, 2015 at 02:58:53PM +0200, Daniel Vetter wrote:
> > > On Wed, May 20, 2015 at 12:50:35PM +0200, Philipp Zabel wrote:
> > > > Hi Daniel,
> > > > 
> > > > thank you for the comments.
> > > > 
> > > > Am Dienstag, den 19.05.2015, 18:52 +0200 schrieb Daniel Vetter:
> > > > [...]
> > > > > > @@ -5198,11 +5206,15 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> > > > > >  		break;
> > > > > >  	case DRM_FORMAT_RGB565:
> > > > > >  	case DRM_FORMAT_BGR565:
> > > > > > +	case DRM_FORMAT_RGB565_A8:
> > > > > > +	case DRM_FORMAT_BGR565_A8:
> > > > > >  		*depth = 16;
> > > > > >  		*bpp = 16;
> > > > > >  		break;
> > > > > >  	case DRM_FORMAT_RGB888:
> > > > > >  	case DRM_FORMAT_BGR888:
> > > > > > +	case DRM_FORMAT_RGB888_A8:
> > > > > > +	case DRM_FORMAT_BGR888_A8:
> > > > > >  		*depth = 24;
> > > > > >  		*bpp = 24;
> > > > > >  		break;
> > > > > > @@ -5210,6 +5222,10 @@ void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> > > > > >  	case DRM_FORMAT_XBGR8888:
> > > > > >  	case DRM_FORMAT_RGBX8888:
> > > > > >  	case DRM_FORMAT_BGRX8888:
> > > > > > +	case DRM_FORMAT_XRGB8888_A8:
> > > > > > +	case DRM_FORMAT_XBGR8888_A8:
> > > > > > +	case DRM_FORMAT_RGBX8888_A8:
> > > > > > +	case DRM_FORMAT_BGRX8888_A8:
> > > > > >  		*depth = 24;
> > > > > >  		*bpp = 32;
> > > > > >  		break;
> > > > > 
> > > > > Please drop the above two hunks, these functions are only for backwards
> > > > > compat with drivers from the addfb1 days. Modern drivers should only use
> > > > > the format tags directly. Extending the plane_cpp function like you do
> > > > > below is enough.
> > > > 
> > > > I'll leave drm_fb_get_bpp_depth untouched.
> > > > 
> > > > > Maybe we should add a WARN_ON(num_planes(format) != 0) to the top of this
> > > > > and a comment that this is for legacy stuff only.
> > > > 
> > > > Do you mean:
> > > > 
> > > > -----8<-----
> > > > --- a/drivers/gpu/drm/drm_crtc.c
> > > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > > @@ -5186,6 +5186,12 @@ int drm_mode_destroy_dumb_ioctl(struct drm_device *dev,
> > > >  void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
> > > >  			  int *bpp)
> > > >  {
> > > > +	/*
> > > > +	 * This function is to be used for legacy drivers only, no new formats
> > > > +	 * should be added here.
> > > > +	 */
> > > > +	WARN_ON(drm_format_num_planes(format) != 1);
> > > > +
> > > >  	switch (format) {
> > > >  	case DRM_FORMAT_C8:
> > > >  	case DRM_FORMAT_RGB332:
> > > > ----->8-----
> > > 
> > > Yeah that looks perfect, please wrap with commit message&sob and I'll
> > > apply.
> > 
> > I think that's going to trigger a lot. IIRC we call this thing for
> > any format when constructing FBs.
> 
> Yes I just noticed that too, for example via:
> 
> drm_fb_cma_create -> drm_fb_cma_alloc -> drm_helper_mode_fill_fb_struct
> -> drm_fb_get_bpp_depth

Hm right, so perhaps just a comment then?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel