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
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;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
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
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
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.
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
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
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Philipp Zabel -
Ville Syrjälä